[Feature] table sample #52600

murphyatwork · 2024-11-05T02:02:47Z

Why I'm doing:

What I'm doing:

Introduce a syntax for table sample:

SELECT count(k1), sum(k1)
FROM t1  SAMPLE('seed'='1', 'method'='by_page', 'percent'='1');

Properties:

seed: optional, specify the random seed, which can guarantee the deterministic result
method: optional, by_block or by_page
percent: (0, 100), sample probability

Fixes #52787

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

satanson · 2024-11-11T11:21:27Z

be/src/storage/rowset/column_iterator.h

@@ -259,9 +261,11 @@ class ColumnIterator {

    virtual Status null_count(size_t* count) { return Status::OK(); };

+    // RAW interface, should be used carefully
+    virtual ColumnReader* get_column_reader() { return nullptr; };


suggest to

virtual ColumnReader* get_column_reader()=0;

or use std::optional<ColumnReader*> as a return type to tell sb that the return type must be checked

add an check in it, actually the unsupported ColumnIterator should not be called.

satanson · 2024-11-11T11:32:19Z

be/src/storage/rowset/data_sample.cpp

+    std::bernoulli_distribution dist(_probability_percent / 100.0);
+
+    size_t sampled_blocks = 0;
+    size_t total_blocks = _total_rows / _rows_per_block;


how _rows_per_block is deduced? it seems _rows_per_block is deduced from rows per block

it's retried from Segment::num_rows_per_block, which is persisted in segment footer.
and also, it can be configured through config.default_num_rows_per_column_file_block, whose default value is 1024.

satanson · 2024-11-11T11:38:57Z

be/src/storage/rowset/data_sample.cpp

+    for (size_t i = 0; i < _total_rows; i += _rows_per_block) {
+        if (dist(mt)) {
+            sampled_blocks++;
+            sampled_ranges.add(RowIdRange(i, std::min(i + _rows_per_block, _total_rows)));


can RowIdRange span two blocks ? a interesting question is what's the math expect of number of RowIdRanges spanning two block when stride(_rows_per_block) varies. is there an optimized stride?

i would say not, each RowIdRange is expected to cover exactly one block

satanson · 2024-11-11T11:50:44Z

be/src/storage/rowset/data_sample.cpp

+
+double SortableZoneMap::width(const Datum& lhs, const Datum& rhs) {
+    if (lhs.is_null() || rhs.is_null()) {
+        return 0;


why is 0, it seems that is a open range, it seens that infinity or maximum value is reasonable

it depends.
here the width is used to construct the histogram, the null value cannot be put into the histogram, so i just ignore them via set the width to 0

Signed-off-by: Murphy <[email protected]>

sonarcloud · 2024-11-14T09:32:06Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

github-actions · 2024-11-14T12:55:58Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-11-14T12:57:50Z

[BE Incremental Coverage Report]

❌ fail : 166 / 324 (51.23%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/storage/rowset/column_iterator.h	0	3	00.00%	[161, 265, 266]
🔵	be/src/storage/rowset/ordinal_page_index.cpp	0	4	00.00%	[186, 187, 188, 189]
🔵	be/src/storage/rowset/scalar_column_iterator.h	0	1	00.00%	[68]
🔵	be/src/exec/pipeline/scan/olap_chunk_source.cpp	0	10	00.00%	[239, 784, 785, 786, 787, 788, 790, 792, 794, 796]
🔵	be/src/storage/rowset/column_reader.cpp	0	17	00.00%	[563, 564, 565, 588, 589, 590, 591, 593, 594, 595, 597, 598, 599, 600, 601, 603, 604]
🔵	be/src/storage/rowset/data_sample.h	1	18	05.56%	[39, 41, 42, 44, 46, 49, 51, 61, 63, 64, 77, 100, 102, 103, 104, 105, 109]
🔵	be/src/storage/rowset/segment_iterator.cpp	4	62	06.45%	[2069, 2070, 2071, 2072, 2073, 2075, 2076, 2077, 2079, 2080, 2082, 2083, 2084, 2085, 2086, 2087, 2089, 2090, 2091, 2093, 2094, 2095, 2096, 2097, 2098, 2100, 2101, 2105, 2106, 2107, 2109, 2110, 2111, 2113, 2115, 2116, 2117, 2118, 2119, 2120, 2121, 2122, 2123, 2124, 2125, 2126, 2127, 2129, 2131, 2134, 2135, 2136, 2137, 2138, 2139, 2140, 2141, 2142]
🔵	be/src/storage/rowset/data_sample.cpp	158	206	76.70%	[47, 48, 49, 50, 64, 67, 82, 94, 97, 99, 117, 118, 127, 128, 129, 130, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 330, 331, 332, 333]
🔵	be/src/storage/rowset/rowset.cpp	1	1	100.00%	[]
🔵	be/src/storage/tablet_reader.cpp	1	1	100.00%	[]
🔵	be/src/storage/zone_map_detail.h	1	1	100.00%	[]

github-actions · 2024-11-14T12:58:44Z

[FE Incremental Coverage Report]

❌ fail : 15 / 85 (17.65%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	com/starrocks/sql/ast/TableSampleClause.java	0	52	00.00%	[44, 48, 49, 50, 51, 53, 54, 55, 58, 63, 67, 68, 69, 71, 72, 75, 76, 77, 79, 80, 82, 83, 85, 87, 88, 91, 92, 93, 94, 95, 99, 100, 101, 102, 103, 104, 108, 109, 110, 111, 112, 115, 116, 117, 118, 120, 121, 126, 127, 128, 130, 134]
🔵	com/starrocks/common/util/ParseUtil.java	0	6	00.00%	[102, 103, 104, 110, 111, 112]
🔵	com/starrocks/sql/ast/TableRelation.java	1	3	33.33%	[230, 231]
🔵	com/starrocks/planner/OlapScanNode.java	4	9	44.44%	[946, 1116, 1117, 1118, 1177]
🔵	com/starrocks/sql/optimizer/operator/physical/PhysicalOlapScanOperator.java	3	6	50.00%	[211, 212, 254]
🔵	com/starrocks/sql/optimizer/operator/logical/LogicalOlapScanOperator.java	5	7	71.43%	[185, 186]
🔵	com/starrocks/sql/optimizer/transformer/RelationTransformer.java	1	1	100.00%	[]
🔵	com/starrocks/sql/plan/PlanFragmentBuilder.java	1	1	100.00%	[]

wanpengfei-git added the PROTO-REVIEW label Nov 5, 2024

wanpengfei-git requested a review from a team November 5, 2024 02:03

mergify bot assigned murphyatwork Nov 5, 2024

murphyatwork force-pushed the murphy_feat_table_sample branch 5 times, most recently from 64435a6 to 46bfe29 Compare November 8, 2024 07:07

murphyatwork marked this pull request as ready for review November 11, 2024 06:28

murphyatwork requested review from a team as code owners November 11, 2024 06:28

murphyatwork force-pushed the murphy_feat_table_sample branch from acf9bfe to bc37039 Compare November 11, 2024 08:43

satanson reviewed Nov 11, 2024

View reviewed changes

murphyatwork added 13 commits November 14, 2024 15:45

tmp

77f85e2

fe

ff3704e

Signed-off-by: Murphy <[email protected]>

be

339e564

Signed-off-by: Murphy <[email protected]>

mvp

cbbc062

Signed-off-by: Murphy <[email protected]>

sample property

d641a8c

Signed-off-by: Murphy <[email protected]>

sample options

03405f2

Signed-off-by: Murphy <[email protected]>

fix thrift

977998e

Signed-off-by: Murphy <[email protected]>

be

19572fb

Signed-off-by: Murphy <[email protected]>

tmp page sample

7a1ef3f

Signed-off-by: Murphy <[email protected]>

tmp page sample

4402c90

Signed-off-by: Murphy <[email protected]>

refactor

fb5b4ef

Signed-off-by: Murphy <[email protected]>

histogram sample

c557bb2

Signed-off-by: Murphy <[email protected]>

add alias PERCENT

5aecdf1

Signed-off-by: Murphy <[email protected]>

murphyatwork added 7 commits November 14, 2024 15:45

profile

3ce8431

Signed-off-by: Murphy <[email protected]>

test case

ce978cc

Signed-off-by: Murphy <[email protected]>

benchmark

b3982a2

Signed-off-by: Murphy <[email protected]>

skip unsupported data types

c550283

Signed-off-by: Murphy <[email protected]>

refine page sample

503537e

Signed-off-by: Murphy <[email protected]>

sql test

c529a80

Signed-off-by: Murphy <[email protected]>

fix

aada2d3

Signed-off-by: Murphy <[email protected]>

murphyatwork force-pushed the murphy_feat_table_sample branch from bc37039 to a958475 Compare November 14, 2024 07:48

fix comment

fffcb42

Signed-off-by: Murphy <[email protected]>

murphyatwork force-pushed the murphy_feat_table_sample branch from 5eb8220 to fffcb42 Compare November 14, 2024 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] table sample #52600

[Feature] table sample #52600

murphyatwork commented Nov 5, 2024 •

edited

Loading

satanson Nov 11, 2024

murphyatwork Nov 14, 2024

satanson Nov 11, 2024

murphyatwork Nov 14, 2024

satanson Nov 11, 2024

murphyatwork Nov 14, 2024

satanson Nov 11, 2024

murphyatwork Nov 14, 2024

sonarcloud bot commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

[Feature] table sample #52600

Are you sure you want to change the base?

[Feature] table sample #52600

Conversation

murphyatwork commented Nov 5, 2024 • edited Loading

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 14, 2024

Quality Gate failed

github-actions bot commented Nov 14, 2024

[Java-Extensions Incremental Coverage Report]

github-actions bot commented Nov 14, 2024

[BE Incremental Coverage Report]

file detail

github-actions bot commented Nov 14, 2024

[FE Incremental Coverage Report]

file detail

murphyatwork commented Nov 5, 2024 •

edited

Loading